From dd2c5980b4bb7d3cf26a6d6c77c7123155bb87cb Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 29 Jul 2014 11:27:40 -0700 Subject: [PATCH] Implement per-target fingerprints This commit refines the granularity of fingerprints from packages to targets, building on the existing infrastructure. All invocations of rustc now include the --dep-info flag which will emit makefile-like dependency information. This information is then parsed later on to determine if all the files are fresh. The purpose of this commit is to refine the set of files which indicate that a target needs to be re-built. Before this commit if you modified a test it would rebuild the entire library, but after this commit it only rebuilds the relevant test. Closes #214 Closes #289 --- src/cargo/ops/cargo_rustc/context.rs | 14 ---- src/cargo/ops/cargo_rustc/fingerprint.rs | 85 ++++++++++++++++------ src/cargo/ops/cargo_rustc/mod.rs | 12 ++-- src/cargo/util/dependency_queue.rs | 2 +- tests/test_cargo_compile.rs | 77 ++++++++------------ tests/test_cargo_cross_compile.rs | 1 + tests/test_cargo_freshness.rs | 92 ++++++++++++++++++++++++ tests/tests.rs | 2 + 8 files changed, 199 insertions(+), 86 deletions(-) create mode 100644 tests/test_cargo_freshness.rs diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 472ca3484..4529c823f 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -238,18 +238,4 @@ impl PlatformRequirement { _ => PluginAndTarget, } } - - pub fn is_target(&self) -> bool { - match *self { - Target | PluginAndTarget => true, - Plugin => false - } - } - - pub fn is_plugin(&self) -> bool { - match *self { - Plugin | PluginAndTarget => true, - Target => false - } - } } diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index e41ca3987..ef0115a7c 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -1,6 +1,6 @@ use std::hash::{Hash, Hasher}; use std::hash::sip::SipHasher; -use std::io::{fs, File, UserRWX}; +use std::io::{fs, File, UserRWX, BufferedReader}; use core::{Package, Target}; use util; @@ -44,31 +44,37 @@ pub fn prepare_target(cx: &mut Context, pkg: &Package, target: &Target, let _p = profile::start(format!("fingerprint: {} / {}", pkg.get_package_id(), target)); let (old, new) = dirs(cx, pkg, kind); - let filename = if target.is_lib() { - format!("lib-{}", target.get_name()) - } else if target.get_profile().is_doc() { - format!("doc-{}", target.get_name()) - } else { - format!("bin-{}", target.get_name()) - }; + let filename = filename(target); let old_loc = old.join(filename.as_slice()); let new_loc = new.join(filename.as_slice()); + let doc = target.get_profile().is_doc(); - let new_fingerprint = try!(calculate_target_fingerprint(cx, pkg, target)); - let new_fingerprint = mk_fingerprint(cx, &new_fingerprint); + // First bit of the freshness calculation, whether the dep-info file + // indicates that the target is fresh. + let (old_dep_info, new_dep_info) = dep_info_loc(cx, pkg, target, kind); + let are_files_fresh = doc || try!(calculate_target_fresh(pkg, &old_dep_info)); + + // Second bit of the freshness calculation, whether rustc itself and the + // target are fresh. + let rustc_fingerprint = if doc { + mk_fingerprint(cx, &(target, try!(calculate_pkg_fingerprint(cx, pkg)))) + } else { + mk_fingerprint(cx, target) + }; + let is_rustc_fresh = try!(is_fresh(&old_loc, rustc_fingerprint.as_slice())); - let is_fresh = try!(is_fresh(&old_loc, new_fingerprint.as_slice())); let layout = cx.layout(kind); let mut pairs = vec![(old_loc, new_loc.clone())]; - if !target.get_profile().is_doc() { + pairs.push((old_dep_info, new_dep_info)); pairs.extend(cx.target_filenames(target).iter().map(|filename| { let filename = filename.as_slice(); ((layout.old_root().join(filename), layout.root().join(filename))) })); } - Ok(prepare(is_fresh, new_loc, new_fingerprint, pairs)) + Ok(prepare(is_rustc_fresh && are_files_fresh, new_loc, rustc_fingerprint, + pairs)) } /// Prepare the necessary work for the fingerprint of a build command. @@ -146,7 +152,7 @@ fn prepare(is_fresh: bool, loc: Path, fingerprint: String, } /// Return the (old, new) location for fingerprints for a package -pub fn dirs(cx: &mut Context, pkg: &Package, kind: Kind) -> (Path, Path) { +pub fn dirs(cx: &Context, pkg: &Package, kind: Kind) -> (Path, Path) { let dirname = format!("{}-{}", pkg.get_name(), short_hash(pkg.get_package_id())); let dirname = dirname.as_slice(); @@ -155,6 +161,14 @@ pub fn dirs(cx: &mut Context, pkg: &Package, kind: Kind) -> (Path, Path) { (layout.old_fingerprint().join(dirname), layout.fingerprint().join(dirname)) } +/// Returns the (old, new) location for the dep info file of a target. +pub fn dep_info_loc(cx: &Context, pkg: &Package, target: &Target, + kind: Kind) -> (Path, Path) { + let (old, new) = dirs(cx, pkg, kind); + let filename = format!("dep-{}", filename(target)); + (old.join(filename.as_slice()), new.join(filename)) +} + fn is_fresh(loc: &Path, new_fingerprint: &str) -> CargoResult { let mut file = match File::open(loc) { Ok(file) => file, @@ -176,21 +190,50 @@ fn mk_fingerprint(cx: &Context, data: &T) -> String { util::to_hex(hasher.hash(&(&cx.rustc_version, data))) } -fn calculate_target_fingerprint(cx: &Context, pkg: &Package, target: &Target) - -> CargoResult { - let source = cx.sources - .get(pkg.get_package_id().get_source_id()) - .expect("BUG: Missing package source"); +fn calculate_target_fresh(pkg: &Package, dep_info: &Path) -> CargoResult { + let line = match BufferedReader::new(File::open(dep_info)).lines().next() { + Some(Ok(line)) => line, + _ => return Ok(false), + }; + let mtime = try!(fs::stat(dep_info)).modified; + let deps = try!(line.as_slice().splitn(':', 1).skip(1).next().require(|| { + internal(format!("dep-info not in an understood format: {}", + dep_info.display())) + })); + + for file in deps.split(' ').map(|s| s.trim()).filter(|s| !s.is_empty()) { + match fs::stat(&pkg.get_root().join(file)) { + Ok(stat) if stat.modified <= mtime => {} + _ => { debug!("stale: {}", file); return Ok(false) } + } + } - let pkg_fingerprint = try!(source.fingerprint(pkg)); - Ok(pkg_fingerprint + short_hash(target)) + Ok(true) } fn calculate_build_cmd_fingerprint(cx: &Context, pkg: &Package) -> CargoResult { + // TODO: this should be scoped to just the `build` directory, not the entire + // package. + calculate_pkg_fingerprint(cx, pkg) +} + +fn calculate_pkg_fingerprint(cx: &Context, pkg: &Package) -> CargoResult { let source = cx.sources .get(pkg.get_package_id().get_source_id()) .expect("BUG: Missing package source"); source.fingerprint(pkg) } + +fn filename(target: &Target) -> String { + let kind = if target.is_lib() {"lib"} else {"bin"}; + let flavor = if target.get_profile().is_test() { + "test-" + } else if target.get_profile().is_doc() { + "doc-" + } else { + "" + }; + format!("{}{}-{}", flavor, kind, target.get_name()) +} diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index b535de7dd..8e8e80482 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -220,8 +220,8 @@ fn prepare_rustc(package: &Package, target: &Target, crate_types: Vec<&str>, let base = process("rustc", package, cx); let base = build_base_args(base, target, crate_types.as_slice()); - let target_cmd = build_plugin_args(base.clone(), cx, KindTarget); - let plugin_cmd = build_plugin_args(base, cx, KindPlugin); + let target_cmd = build_plugin_args(base.clone(), cx, package, target, KindTarget); + let plugin_cmd = build_plugin_args(base, cx, package, target, KindPlugin); let target_cmd = build_deps_args(target_cmd, target, package, cx, KindTarget); let plugin_cmd = build_deps_args(plugin_cmd, target, package, cx, KindPlugin); @@ -315,15 +315,19 @@ fn build_base_args(mut cmd: ProcessBuilder, } None => {} } + return cmd; } -fn build_plugin_args(mut cmd: ProcessBuilder, cx: &Context, - kind: Kind) -> ProcessBuilder { +fn build_plugin_args(mut cmd: ProcessBuilder, cx: &Context, pkg: &Package, + target: &Target, kind: Kind) -> ProcessBuilder { cmd = cmd.arg("--out-dir"); cmd = cmd.arg(cx.layout(kind).root()); + let (_, dep_info_loc) = fingerprint::dep_info_loc(cx, pkg, target, kind); + cmd = cmd.arg("--dep-info").arg(dep_info_loc); + if kind == KindTarget { fn opt(cmd: ProcessBuilder, key: &str, prefix: &str, val: Option<&str>) -> ProcessBuilder { diff --git a/src/cargo/util/dependency_queue.rs b/src/cargo/util/dependency_queue.rs index d8b51abc9..28070e299 100644 --- a/src/cargo/util/dependency_queue.rs +++ b/src/cargo/util/dependency_queue.rs @@ -36,7 +36,7 @@ pub struct DependencyQueue { /// /// A fresh package does not necessarily need to be rebuilt (unless a dependency /// was also rebuilt), and a dirty package must always be rebuilt. -#[deriving(PartialEq)] +#[deriving(PartialEq, Eq, Show)] pub enum Freshness { Fresh, Dirty, diff --git a/tests/test_cargo_compile.rs b/tests/test_cargo_compile.rs index 32e57fb46..d7506e454 100644 --- a/tests/test_cargo_compile.rs +++ b/tests/test_cargo_compile.rs @@ -1,7 +1,6 @@ use std::io::{fs, TempDir}; use std::os; use std::path; -use std::str; use support::{ResultTest, project, execs, main_file, basic_bin_manifest}; use support::{COMPILING, RUNNING, cargo_dir, ProjectBuilder}; @@ -1091,23 +1090,19 @@ test!(verbose_build { authors = [] "#) .file("src/lib.rs", ""); - let output = p.cargo_process("cargo-build").arg("-v") - .exec_with_output().assert(); - let out = str::from_utf8(output.output.as_slice()).assert(); - let hash = out.slice_from(out.find_str("extra-filename=").unwrap() + 16); - let hash = hash.slice_to(16); - assert_eq!(out, format!("\ -{} `rustc {dir}{sep}src{sep}lib.rs --crate-name test --crate-type lib \ - -C metadata={hash} \ - -C extra-filename=-{hash} \ + assert_that(p.cargo_process("cargo-build").arg("-v"), + execs().with_status(0).with_stdout(format!("\ +{running} `rustc {dir}{sep}src{sep}lib.rs --crate-name test --crate-type lib \ + -C metadata=[..] \ + -C extra-filename=-[..] \ --out-dir {dir}{sep}target \ + --dep-info [..] \ -L {dir}{sep}target \ -L {dir}{sep}target{sep}deps` -{} test v0.0.0 (file:{dir})\n", - RUNNING, COMPILING, - dir = p.root().display(), - sep = path::SEP, - hash = hash).as_slice()); +{compiling} test v0.0.0 (file:{dir})\n", +running = RUNNING, compiling = COMPILING, sep = path::SEP, +dir = p.root().display() +))); }) test!(verbose_release_build { @@ -1121,25 +1116,21 @@ test!(verbose_release_build { authors = [] "#) .file("src/lib.rs", ""); - let output = p.cargo_process("cargo-build").arg("-v").arg("--release") - .exec_with_output().assert(); - let out = str::from_utf8(output.output.as_slice()).assert(); - let hash = out.slice_from(out.find_str("extra-filename=").unwrap() + 16); - let hash = hash.slice_to(16); - assert_eq!(out, format!("\ -{} `rustc {dir}{sep}src{sep}lib.rs --crate-name test --crate-type lib \ + assert_that(p.cargo_process("cargo-build").arg("-v").arg("--release"), + execs().with_status(0).with_stdout(format!("\ +{running} `rustc {dir}{sep}src{sep}lib.rs --crate-name test --crate-type lib \ --opt-level 3 \ --cfg ndebug \ - -C metadata={hash} \ - -C extra-filename=-{hash} \ + -C metadata=[..] \ + -C extra-filename=-[..] \ --out-dir {dir}{sep}target{sep}release \ + --dep-info [..] \ -L {dir}{sep}target{sep}release \ -L {dir}{sep}target{sep}release{sep}deps` -{} test v0.0.0 (file:{dir})\n", - RUNNING, COMPILING, - dir = p.root().display(), - sep = path::SEP, - hash = hash).as_slice()); +{compiling} test v0.0.0 (file:{dir})\n", +running = RUNNING, compiling = COMPILING, sep = path::SEP, +dir = p.root().display() +))); }) test!(verbose_release_build_deps { @@ -1168,44 +1159,38 @@ test!(verbose_release_build_deps { crate_type = ["dylib", "rlib"] "#) .file("foo/src/lib.rs", ""); - let output = p.cargo_process("cargo-build").arg("-v").arg("--release") - .exec_with_output().assert(); - let out = str::from_utf8(output.output.as_slice()).assert(); - let pos1 = out.find_str("extra-filename=").unwrap(); - let hash1 = out.slice_from(pos1 + 16).slice_to(16); - let pos2 = out.slice_from(pos1 + 10).find_str("extra-filename=").unwrap(); - let hash2 = out.slice_from(pos1 + 10 + pos2 + 16).slice_to(16); - assert_eq!(out, format!("\ + assert_that(p.cargo_process("cargo-build").arg("-v").arg("--release"), + execs().with_status(0).with_stdout(format!("\ {running} `rustc {dir}{sep}foo{sep}src{sep}lib.rs --crate-name foo \ --crate-type dylib --crate-type rlib \ --opt-level 3 \ --cfg ndebug \ - -C metadata={hash1} \ - -C extra-filename=-{hash1} \ + -C metadata=[..] \ + -C extra-filename=-[..] \ --out-dir {dir}{sep}target{sep}release{sep}deps \ + --dep-info [..] \ -L {dir}{sep}target{sep}release{sep}deps \ -L {dir}{sep}target{sep}release{sep}deps` {running} `rustc {dir}{sep}src{sep}lib.rs --crate-name test --crate-type lib \ --opt-level 3 \ --cfg ndebug \ - -C metadata={hash2} \ - -C extra-filename=-{hash2} \ + -C metadata=[..] \ + -C extra-filename=-[..] \ --out-dir {dir}{sep}target{sep}release \ + --dep-info [..] \ -L {dir}{sep}target{sep}release \ -L {dir}{sep}target{sep}release{sep}deps \ --extern foo={dir}{sep}target{sep}release{sep}deps/\ - {prefix}foo-{hash1}{suffix} \ - --extern foo={dir}{sep}target{sep}release{sep}deps/libfoo-{hash1}.rlib` + {prefix}foo-[..]{suffix} \ + --extern foo={dir}{sep}target{sep}release{sep}deps/libfoo-[..].rlib` {compiling} foo v0.0.0 (file:{dir}) {compiling} test v0.0.0 (file:{dir})\n", running = RUNNING, compiling = COMPILING, dir = p.root().display(), sep = path::SEP, - hash1 = hash1, - hash2 = hash2, prefix = os::consts::DLL_PREFIX, - suffix = os::consts::DLL_SUFFIX).as_slice()); + suffix = os::consts::DLL_SUFFIX).as_slice())); }) test!(explicit_examples { diff --git a/tests/test_cargo_cross_compile.rs b/tests/test_cargo_cross_compile.rs index f5c4601c3..eeb14b445 100644 --- a/tests/test_cargo_cross_compile.rs +++ b/tests/test_cargo_cross_compile.rs @@ -258,6 +258,7 @@ test!(linker_and_ar { .with_stdout(format!("\ {running} `rustc src/foo.rs --crate-name foo --crate-type bin \ --out-dir {dir}{sep}target{sep}{target} \ + --dep-info [..] \ --target {target} \ -C ar=my-ar-tool -C linker=my-linker-tool \ -L {dir}{sep}target{sep}{target} \ diff --git a/tests/test_cargo_freshness.rs b/tests/test_cargo_freshness.rs new file mode 100644 index 000000000..94bd19a05 --- /dev/null +++ b/tests/test_cargo_freshness.rs @@ -0,0 +1,92 @@ +use std::io::{fs, File}; +use time; + +use support::{project, execs}; +use support::{COMPILING, cargo_dir, ResultTest, FRESH}; +use hamcrest::{assert_that, existing_file}; + +fn setup() {} + +test!(modifying_and_moving { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + authors = [] + version = "0.0.1" + "#) + .file("src/main.rs", r#" + mod a; fn main() {} + "#) + .file("src/a.rs", ""); + + assert_that(p.cargo_process("cargo-build"), + execs().with_status(0).with_stdout(format!("\ +{compiling} foo v0.0.1 (file:{dir}) +", compiling = COMPILING, dir = p.root().display()))); + + assert_that(p.process(cargo_dir().join("cargo-build")), + execs().with_status(0).with_stdout(format!("\ +{fresh} foo v0.0.1 (file:{dir}) +", fresh = FRESH, dir = p.root().display()))); + + File::create(&p.root().join("src/a.rs")).write_str("fn main() {}").assert(); + assert_that(p.process(cargo_dir().join("cargo-build")), + execs().with_status(0).with_stdout(format!("\ +{compiling} foo v0.0.1 (file:{dir}) +", compiling = COMPILING, dir = p.root().display()))); + + fs::rename(&p.root().join("src/a.rs"), &p.root().join("src/b.rs")).assert(); + assert_that(p.process(cargo_dir().join("cargo-build")), + execs().with_status(101)); +}) + +test!(modify_only_some_files { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + authors = [] + version = "0.0.1" + "#) + .file("src/lib.rs", "mod a;") + .file("src/a.rs", "") + .file("src/main.rs", r#" + mod b; + fn main() {} + "#) + .file("src/b.rs", "") + .file("tests/test.rs", ""); + + assert_that(p.cargo_process("cargo-build"), + execs().with_status(0).with_stdout(format!("\ +{compiling} foo v0.0.1 (file:{dir}) +", compiling = COMPILING, dir = p.root().display()))); + assert_that(p.process(cargo_dir().join("cargo-test")), + execs().with_status(0)); + + assert_that(&p.bin("foo"), existing_file()); + + let past = time::precise_time_ns() / 1_000_000 - 5000; + + let lib = p.root().join("src/lib.rs"); + let bin = p.root().join("src/b.rs"); + let test = p.root().join("tests/test.rs"); + + File::create(&lib).write_str("invalid rust code").assert(); + fs::change_file_times(&lib, past, past).assert(); + + File::create(&bin).write_str("fn foo() {}").assert(); + + // Make sure the binary is rebuilt, not the lib + assert_that(p.process(cargo_dir().join("cargo-build")), + execs().with_status(0).with_stdout(format!("\ +{compiling} foo v0.0.1 (file:{dir}) +", compiling = COMPILING, dir = p.root().display()))); + assert_that(&p.bin("foo"), existing_file()); + + // Make sure the tests don't recompile the lib + File::create(&test).write_str("fn foo() {}").assert(); + assert_that(p.process(cargo_dir().join("cargo-test")), + execs().with_status(0)); +}) diff --git a/tests/tests.rs b/tests/tests.rs index 37a7364c6..f86a631e5 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1,6 +1,7 @@ #![feature(macro_rules)] #![feature(phase)] +extern crate time; extern crate term; extern crate cargo; extern crate hamcrest; @@ -32,3 +33,4 @@ mod test_cargo_version; mod test_cargo_new; mod test_cargo_compile_plugins; mod test_cargo_doc; +mod test_cargo_freshness; -- 2.30.2